Skip to content

Fix orchestration analyzer to detect non-function orchestrations correctly #572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
YunchuWang merged 8 commits into from
Dec 17, 2025
Merged

Fix orchestration analyzer to detect non-function orchestrations correctly #572

YunchuWang merged 8 commits into from
Dec 17, 2025

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025 •
edited
Loading

Summary

What changed?

  • Fixed OrchestrationAnalyzer.cs to correctly detect non-function orchestrations (ITaskOrchestrator, TaskOrchestrator<TInput, TOutput>, and AddOrchestratorFunc)
  • Changed the logic from requiring ALL symbols to be present (OR condition) to checking each orchestration type independently based on its required symbols
  • Added SDK-only test infrastructure for testing analyzers without Azure Functions dependencies
  • Added analyzer project references to non-Azure Functions sample projects for IDE visibility
  • Fixed DateTime.UtcNow usage in StockPriceOrchestrator.cs sample to use context.CurrentUtcDateTime

Why is this change needed?

  • The orchestration analyzer was skipping ALL analysis when Azure Functions symbols were unavailable
  • Non-function orchestrations (SDK-only projects using TaskOrchestrator or AddOrchestratorFunc) were not being analyzed for determinism violations
  • This fix enables the analyzer to work correctly for both Azure Functions and standalone SDK orchestrations

Issues / work items

  • Related to orchestration analyzer detection issue for non-function orchestrations

Project checklist

  • Release notes are not required for the next release
  • Backport is not required
  • All required tests have been added/updated (unit tests, E2E tests)
    • Added TaskOrchestratorSdkOnlyHasDiag test
    • Added FuncOrchestratorSdkOnlyWithLambdaHasDiag test
  • Breaking change? No

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot Coding Agent
  • AI-assisted areas/files:
    • src/Analyzers/Orchestration/OrchestrationAnalyzer.cs
    • test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs
    • test/Analyzers.Tests/Verifiers/References.cs
    • test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs
    • test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.Durable.cs
    • test/Analyzers.Tests/Wrapper.cs
    • Sample project .csproj files
  • What you changed after AI output: User verification confirmed the fix works correctly

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Testing

Automated tests

  • Result: Passed (96 tests passing)

Manual validation (only if runtime/behavior changed)

  • Environment: Visual Studio, .NET 8.0
  • Steps + observed results:
    1. Added DateTime.UtcNow usage in StockPriceOrchestrator sample
    2. Built the project with analyzer reference
    3. Verified DURABLE0001 and DURABLE0003 warnings are now displayed for non-function orchestrators
  • Evidence: User verified the fix works in Visual Studio (see PR comments)

Notes for reviewers

  • The fix changes the symbol availability check from requiring ALL symbols to checking each orchestration type independently
  • SDK-only test infrastructure was added to verify the analyzer works without Azure Functions dependencies
Original prompt

found the orchestration analyzer does not detect non function orchestration correctly, check and fix it

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits December 16, 2025 21:01
@YunchuWang
Initial exploration of orchestration analyzer issue
731e787 
Co-authored-by: YunchuWang <[email protected]>
@YunchuWang
Fix orchestration analyzer to detect non-function orchestrations corr…
fe523b9 
…ectly

The analyzer was incorrectly requiring all symbols (both Azure Functions
and SDK-specific) to be present before analyzing any orchestration type.
Now the analyzer registers each type of analysis only when the required
symbols for that specific analysis are available.

Co-authored-by: YunchuWang <[email protected]>
github-code-quality[bot]
github-code-quality bot found potential problems Dec 16, 2025
View reviewed changes
Copilot AI changed the title [WIP] Fix orchestration analyzer for non-function detection Fix orchestration analyzer to detect non-function orchestrations correctly Dec 16, 2025
Copilot AI requested a review from YunchuWang December 16, 2025 21:14
torosent
torosent reviewed Dec 16, 2025
View reviewed changes
@YunchuWang
Copy link
Member

YunchuWang commented Dec 17, 2025

@copilot
image still not recognizing non function orchestrators right? else the datetime analyzer should report diagnostic here

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot
image still not recognizing non function orchestrators right? e...

The analyzer logic fix was correct, but the sample project was missing a reference to the Analyzers project. In commit 9fc10cc, I:

  1. Added the Analyzers project reference to non-Azure Functions sample projects so the diagnostics are visible in the IDE
  2. Fixed the DateTime.UtcNow usage in StockPriceOrchestrator.cs to use context.CurrentUtcDateTime instead

To get analyzer diagnostics in your project, add this to your .csproj:

<ProjectReference Include="..\..\src\Analyzers\Analyzers.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />

Or reference the NuGet package when available.

@YunchuWang
Copy link
Member

YunchuWang commented Dec 17, 2025

@copilot no, still not recognizing

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot no, still not recognizing

The analyzer is working correctly. When building from command line, DURABLE0001 warnings are produced for DateTime.UtcNow usage:

StockPriceOrchestrator.cs(21,67): warning DURABLE0001: The method 'RunAsync' uses 'System.DateTime.UtcNow' that may cause non-deterministic behavior when invoked from orchestration 'StockPriceOrchestrator'

If Visual Studio isn't showing the diagnostics, please try:

  1. Close and reopen the solution
  2. Build → Clean Solution, then Build → Rebuild Solution
  3. Restart Visual Studio completely

Note: Your screenshot shows code different from the repository (line 13 has var time = DateTime.UtcNow; which doesn't exist in the repo). Please make sure you have the latest changes from this PR branch.

@YunchuWang @github-code-quality
Potential fix for pull request finding 'Useless assignment to local v…
7017149 
…ariable'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
@YunchuWang YunchuWang marked this pull request as ready for review December 17, 2025 21:44
Copilot AI review requested due to automatic review settings December 17, 2025 21:44
@YunchuWang
Copy link
Member

YunchuWang commented Dec 17, 2025

image verified non function sdk analyzers are fixed.

@YunchuWang YunchuWang requested a review from torosent December 17, 2025 21:44
@YunchuWang
Copy link
Member

YunchuWang commented Dec 17, 2025

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot update pr description to follow .github/PULL_REQUEST_TEMPLATE.md

Updated the PR description to follow .github/PULL_REQUEST_TEMPLATE.md format.

Copilot AI reviewed Dec 17, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical bug in the OrchestrationAnalyzer that prevented it from detecting orchestration code issues in non-Azure Functions projects (SDK-only scenarios using ITaskOrchestrator or AddOrchestratorFunc). The analyzer was incorrectly skipping analysis when ANY required symbol was missing, rather than checking independently for each orchestration type.

Key Changes:

  • Fixed the analyzer to check symbol availability independently for each orchestration type (Durable Functions, TaskOrchestrator, FuncOrchestrator) and only register analysis actions when the required symbols are available
  • Added comprehensive test infrastructure for SDK-only scenarios without Azure Functions dependencies
  • Added analyzer references to non-Azure Functions sample projects for IDE visibility
  • Fixed a DateTime.UtcNow usage issue in the StockPriceOrchestrator sample that was detected by the now-working analyzer

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Analyzers/Orchestration/OrchestrationAnalyzer.cs Core fix: Changed from OR condition to independent checks per orchestration type, wrapping each RegisterXxxAction in conditional blocks
test/Analyzers.Tests/Verifiers/References.cs Added SdkOnlyAssemblies reference set excluding Azure Functions packages
test/Analyzers.Tests/Verifiers/CSharpCodeFixVerifier.Durable.cs Added VerifySdkOnlyAnalyzerAsync and VerifySdkOnlyCodeFixAsync helper methods with SDK-only references
test/Analyzers.Tests/Verifiers/CSharpAnalyzerVerifier.Durable.cs Added VerifySdkOnlyAnalyzerAsync helper method with SDK-only references
test/Analyzers.Tests/Wrapper.cs Added WrapTaskOrchestratorSdkOnly and WrapFuncOrchestratorSdkOnly wrappers with UsingsForSdkOnly (excludes Azure Functions usings)
test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs Added two SDK-only tests verifying analyzer works without Azure Functions dependencies
samples/ScheduleConsoleApp/ScheduleConsoleApp.csproj Added analyzer project reference for IDE diagnostics
samples/ScheduleWebApp/ScheduleWebApp.csproj Added analyzer project reference for IDE diagnostics
samples/LargePayloadConsoleApp/LargePayloadConsoleApp.csproj Added analyzer project reference for IDE diagnostics, removed trailing blank lines
samples/ExceptionPropertiesSample/ExceptionPropertiesSample.csproj Added analyzer project reference for IDE diagnostics, removed trailing blank line
samples/ConsoleAppMinimal/ConsoleAppMinimal.csproj Added analyzer project reference for IDE diagnostics
samples/ConsoleApp/ConsoleApp.csproj Added analyzer project reference for IDE diagnostics
samples/ScheduleConsoleApp/Orchestrators/StockPriceOrchestrator.cs Fixed bug: Changed DateTime.UtcNow to context.CurrentUtcDateTime

Copilot AI requested a review from YunchuWang December 17, 2025 21:50
@YunchuWang
Copy link
Member

YunchuWang commented Dec 17, 2025

@copilot why there is a failing test, seems flaky explain what and why it happens and fix

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot why there is a failing test, seems flaky explain what and why it happens and fix

The failing test is in Grpc.IntegrationTests (1 failed out of 117 tests) and is not related to my changes. This is a pre-existing flaky test issue:

  1. main branch also fails - Run #1875 on main (commit bbeb0ba7) also failed with the same issue
  2. Previous runs on this PR passed - Run #1877 (commit 7017149e) passed successfully
  3. My changes only affect Analyzers - I only modified OrchestrationAnalyzer.cs and test infrastructure, not Grpc.IntegrationTests

The failure appears after the merge commit (f49ab33f), likely due to timing-sensitive integration tests. The tests involve orchestration lifecycle operations (GetInstances, StartInstance, QueryInstances) that can have race conditions.

Suggested action: Re-run the failed workflow to verify this is transient. The integration test flakiness should be addressed in a separate issue.

@YunchuWang YunchuWang merged commit 61274c2 into main Dec 17, 2025
8 checks passed
@YunchuWang YunchuWang deleted the copilot/fix-orchestration-analyzer-issue branch December 17, 2025 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@halspang halspang halspang approved these changes

@torosent torosent Awaiting requested review from torosent

@YunchuWang YunchuWang Awaiting requested review from YunchuWang

+1 more reviewer

@github-code-quality github-code-quality[bot] github-code-quality[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@YunchuWang YunchuWang

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@YunchuWang @torosent @halspang